Use UTC timezone for ES indices #646
Conversation
pavolloffay
requested review from
black-adder,
vprithvi and
yurishkuro
as code owners
ghost
assigned 
|
It might also be good if timezone can be overridden by config - with local as the default. |
|
I would prefer to do it in a separate PR. This just fixes inconsistency bug. |
|
I would even prefer no conf prop if it can be configured via the system |
|
it seems this change reverts another one specifically done to introduce the UTC, I think we have a conflict of requirements. |
|
Which one? https://github.com/jaegertracing/jaeger/pulls?q=is%3Apr+utc+is%3Aclosed are related to tests failures only, Probably inconsystency between local and UTC. |
|
I strongly prefer What is the argument for updating to local tz? Jaeger UI can translate these times to the user's current timezone. |
|
I also prefer UTC. Different instances of collectors must all agree on when the index names roll over, so leaving this up to local time zone on the host opens them up to errors due to misconfiguration or regional differences. |
Signed-off-by: Pavol Loffay <[email protected]>
pavolloffay
force-pushed
the
fix-es-index-timezone
branch
from
ba16f14 to
bb3f9bc
Compare
pavolloffay
changed the title
|
It's a good argument when running in multiple regions. I think UTC is a better default, but it's harcoded. Setting to local leaves the opportunity to configure it in the system. However I don't know how people do it in practice. I have updated to UTC. |
|
I remember at my previous job, which was a Java shop, we used to run all Java apps with an env variable setting the timezone to UTC, to avoid surprises of local timezones. |
| @@ -131,7 +131,7 @@ func (s *SpanWriter) WriteSpan(span *model.Span) error { | |||
| } | |||
|
|
|||
| func indexNames(span *model.Span) (string, string) { | |||
There was a problem hiding this comment.
something I don't understand. The issue said there is a discrepancy between reader and writer in the use of UTC, but this function is global. Is the function shared by the reader/writer? If not, I think that's the missed opportunity to remove the discrepancies.
I see that the reader has another function func indexWithDate(prefix string, date time.Time) string {.
So it looks like they both re-implement similar functionality instead of relying on a shared struct doing it for them.
There was a problem hiding this comment.
agree I will address it in a separate PR
Fixes #591
Signed-off-by: Pavol Loffay [email protected]